Generate proper types for untagged unions. #368
Conversation
jmcarp
force-pushed
the
jmcarp/untagged-unions
branch
from
107c4e6 to
de86fcb
Compare
sudomateo
left a comment
sudomateo
left a comment
There was a problem hiding this comment.
This approach looks good to me! I left some comments around a few areas but overall I like the approach.
|
|
||
| func detectIPv6Format(s string) bool { | ||
| ip := net.ParseIP(s) | ||
| return ip != nil && ip.To4() == nil |
There was a problem hiding this comment.
Oh Go. The difference between this and the function above it is quite subtle. I wish there were IsV4 and IsV6 methods or something.
oxide/types.go
Outdated
|
|
||
| func (v IpNet) MarshalJSON() ([]byte, error) { | ||
| if v.Value == nil { | ||
| return []byte("null"), nil |
There was a problem hiding this comment.
Why do we want to explicitly write null here instead of letting Go use the struct field tags omit*?
There was a problem hiding this comment.
I'm getting rid of this part. encoding/json can handle nil values without any special logic here.
There was a problem hiding this comment.
This design makes sense to me! My only question is do we only use this untagged style when the variants are the same type? I know in Rust they aren't the same type but I mean their serialized form. That is, IpNet variants are all string and IpRange are all struct{first, last}.
There was a problem hiding this comment.
The case we're considering here is string fields with either format or pattern in the openapi spec. At the moment, that's how all untagged unions work in nexus, although obviously this could change in the future. In the case where different variants use different types, we could think about attempting to unmarshal the data into each variant, and accepting the first variant type that works.
jmcarp
force-pushed
the
jmcarp/untagged-unions
branch
4 times, most recently
from
ce15434 to
ef79707
Compare
jmcarp
force-pushed
the
jmcarp/untagged-unions
branch
3 times, most recently
from
74117ab to
04cdfe7
Compare
The sdk contains a small number of untagged unions: union types that lack a
discriminator field. Because we don't have a field that tells us which variant
to use, we currently model these types as `interface{}`.
This patch introduces interface types with variant markers for untagged unions
whose fields can be distinguished using OpenAPI `format` or `pattern` metadata.
On unmarshal, check each `format` or `pattern` field against the incoming data,
and hydrate into the first matching variant type.
jmcarp
force-pushed
the
jmcarp/untagged-unions
branch
from
04cdfe7 to
4f2d121
Compare
The sdk contains a small number of untagged unions: union types that lack a discriminator field. Because we don't have a field that tells us which variant to use, we currently model these types as
interface{}.This patch introduces interface types with variant markers for untagged unions whose fields can be distinguished using OpenAPI
formatorpatternmetadata. On unmarshal, check eachformatorpatternfield against the incoming data, and hydrate into the first matching variant type.As for previous code generation changes, review DESIGN.md first, then the generated code. The code generation code is still WIP.